fix: keep stale needs_more_info copy out of trader status after resubmit - #54
Merged
Merged
Conversation
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Aravinda-HWK
force-pushed
the
fix/stale-needs-more-info-after-resubmit
branch
from
September 6, 2026 05:26
d5bdccf to
6261721
Compare
ginaxu1
force-pushed
the
fix/stale-needs-more-info-after-resubmit
branch
from
September 9, 2026 07:03
6261721 to
2a93813
Compare
Contributor
Author
|
Retested, screenrecording shows CDA |
ginaxu1
force-pushed
the
fix/stale-needs-more-info-after-resubmit
branch
5 times, most recently
from
September 11, 2026 04:20
cc91a8e to
efef7e5
Compare
lokewate
approved these changes
Sep 12, 2026
Same-step correction loops were still showing "please resubmit" as current status in QUEUED_EXTERNALLY because leftover officer outcome data was not gated by task state. Split awaiting vs action-required banners the same way as the SLTB blendsheet fix. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
ginaxu1
force-pushed
the
fix/stale-needs-more-info-after-resubmit
branch
from
September 14, 2026 04:52
efef7e5 to
1f47c16
Compare
lokewate
added a commit
that referenced
this pull request
Sep 15, 2026
… status The status-awaiting template merged in from main (#54) referenced traderinput.treatment_type/treatment_duration as top-level fields, but the batch refactor moved them under traderinput.treatment_items[]. This is the only section visible while a request sits in QUEUED_EXTERNALLY, so traders saw two blank lines and no item list while awaiting review. Match the per-item rendering already used in status_markdown.json's own awaiting branch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
lokewate
added a commit
that referenced
this pull request
Sep 15, 2026
) * feat(npqs): add v2 parallel-batch phytosanitary certificate workflow Redesigns the NPQS export flow from one monolithic per-consignment pass into a graph that forks into three concurrent tracks (lab, visual, treatment) via PARALLEL_SPLIT, each partitioning the consignment down to just the items that need it via BATCH_SPLIT/ BATCH_JOIN pairs keyed on the officer's per-item routing decision (lab_required / visual_required / treatment_required), before rejoining for document review, payment, certificate issuance, and ePhyto transmission. Within that: the lab track's result gate (lab_result_split / lab_result_join) is itself a nested per-item BATCH_SPLIT/BATCH_JOIN — one item failing final doesn't hold up others in the same batch, and only items marked for resubmission loop back through a dedicated redraw/retest pair (kept structurally separate from the initial draw/test nodes so the nested batch region stays topologically closed, per the engine's BATCH_SPLIT region-closure validation). The treatment-request and certificate-issuance steps each carry a per-item picker (treatment specs per item; which items make the final certificate) instead of one decision for the whole consignment. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(npqs): show per-item context on the real agency review forms These are the forms officers actually see (task_code -> taskconfig -> forms.review), separate from the tnsw/npqs_v2 spec copies — several were missing the per-item breakdown the spec copies already had, and none of the treatment-track ones showed which items a batch decision actually covered. Adds a read-only/editable per-item array (id + commodity name, plus track-specific fields) to each, marked addable:false/removable:false now that ArrayControl supports it, and adds the missing item context to npqs_treatment_review's view form (previously showed zero commodity context at all — just the trader's raw uploaded cert URLs). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * wip(npqs): application review + treatment request review form updates Pre-existing uncommitted local changes, not authored this session — committing as-is to track them (large diffs, not independently reviewed or verified here). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * wip(trade): manifest and HS code selection updates Pre-existing uncommitted local changes, not authored this session and unrelated to the NPQS workflow — committing as-is to track them, not independently reviewed or verified here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(npqs-v2): fix blank trader-facing status views on visual-inspection tracks The trader-side status markdown for all three visual-inspection tracks (sample, consignment, system) referenced flat top-level fields like .visual_result, .comments, .system_reference_id. Those values are never flat: TaskManager.CompleteTaskStep namespaces every EXTERNAL_REVIEW/ USER_INPUT submission under the subtask template's own output_namespace (record.Data[namespace] = payload) before it reaches the render layer, so the officer's actual submission lives at .reviewerform.visual_result etc. Since the flat key never existed, requireDataKey: "visual_result" never matched post-completion either, so the whole view rendered as {}. Fixes: - render.json: requireDataKey now checks "reviewerform" (the namespace key that's actually present once the officer completes their review), not the never-populated flat "visual_result". - status_markdown.json: templates now read .reviewerform.* for officer-submitted fields, and .traderinput.* (already correct) for trader-submitted scheduling fields. - Every state (pending, scheduled, completed) now lists the batch's commodities up top and, once reviewed, a per-item pass/fail/reason breakdown from .reviewerform.items - not just an aggregate result - so both officer and trader can see exactly which items a decision covers. - 4-1-visual_sample: status_message was ungated (always rendered) while assessment_outcome only rendered once reviewerform existed, causing the same content to render twice post-completion; gated status_message to QUEUED_EXTERNALLY to match the other two tracks. Verified by executing all 6 templates (npqs_v2 + deployed npqs mirrors) through Go's text/template with realistic pending/scheduled/completed data shapes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(npqs-v2): fix the same reviewerform-nesting bug in 6 more trader-facing status views Same root cause as the visual-inspection fix: every EXTERNAL_REVIEW step in NPQS namespaces the officer's submission under record.Data.reviewerform (TaskManager.CompleteTaskStep writes record.Data[output_namespace] = payload), but these six trader-facing status templates still referenced the pre-fix flat field names, so their result sections stayed permanently hidden/empty once an officer actually completed a review: - 3-lab_testing: sample_test_result, lab_comments, lab_report_attachment_url - 5-5-treatment_review_certs: treatment_review_result, review_comments, post_treatment_visual_required - 6-2-review_docs: docs_review_outcome, review_comments - 5-6-treatment_supervisor_report: supervisor_notes, supervision_report_url - 5-3-treatment_cert_issue: treatment_certificate_id, treatment_outcome, issue_comments - 8-issue_certificate: certificate_id, certificate_url, comments, and the certificate_items per-item inclusion list (was reading the pre-decision dispatch-time array, never the officer's actual include_in_certificate choice at .reviewerform.certificate_items) render.json's requireDataKey gates updated the same way (now check for the "reviewerform" namespace key, which is reliably present once the officer completes their review, instead of a flat field that's never populated at that path). Lab testing's per-item breakdown now ranges over the officer's actual submitted .reviewerform.items instead of the static dispatch-time .commodities list, so per-item pass/fail actually reflects what was reviewed. Left unchanged (confirmed correct on inspection): commodity/treatment item context lists that are self-mapped at dispatch time (.commodities, .treatment_items, .certificate_items as pre-decision context, reference_number, sample_number, ephyto_required, treatment_supervision) - those are genuinely flat in record.Data and were never part of this bug. 2-sample_collection was also checked and needs no change: its results_summary section already uses dataKey: "officer_receive" to scope the projector to the right sub-object, which is the same fix applied a different way. Verified by executing all 12 changed template files (npqs_v2 + deployed npqs mirrors) through Go's text/template with realistic pending/completed data shapes for each track. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * revert(npqs): restore tnsw/npqs render/status templates, untouched by npqs_v2 tnsw/npqs is the old, single-item workflow mirror and is actively being tested right now — don't touch it while that's in progress. Reverts the render.json/status_markdown.json edits from the last two commits for this directory only; tnsw/npqs_v2's equivalent fixes stay in place. See the npqs_v2 top-level split in the next commit for how batch-workflow testing is now fully decoupled from this tree going forward. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat(npqs): split a top-level npqs_v2/ artifact root off npqs/ Fully decouples the batch-workflow (tnsw/npqs_v2) officer-facing forms from the old single-item flow's forms, which are still being actively tested against the shared top-level npqs/ tree. Root cause: every EXTERNAL_REVIEW task's plugin_properties.task_code is resolved against the agency backend's artifact registry by task_code as a flat id (npqs/manifest.json). tnsw/npqs and tnsw/npqs_v2 used byte-identical task codes (e.g. npqs_lab_testing_v1) for the same conceptual review step, so both workflow versions resolved to the exact same officerinput_jsonform.json under npqs/ - any edit there (per-item context, form fields, anything) hit both flows at once, with no way to change one in isolation. npqs_v2/ is a full copy of npqs/ (so it starts from the same, already per-item-aware forms) with every id renamed to a distinct "_v2"/"-v2" suffix - task codes, taskconfig filenames, and every generic_template (jsonform) id referenced from forms.view/forms.review - so registering both trees into one registry can't collide. tnsw/npqs_v2/**/*.json's task_code fields (13 EXTERNAL_REVIEW/PAYMENT tracks) now point at the new _v2 codes; tnsw/npqs's task codes are untouched and still resolve into npqs/ exactly as before. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat(npqs-v2): add the missing sample-submission handshake for visual-sample inspection Selecting "sample" as the visual inspection approach routed straight to the officer's inspection review with no way for a physical sample to actually reach NPQS first - no trader submission step, no officer acknowledgement, and n_visual_sample_inspection's own sample_number input silently depended on the (unrelated) lab track's npqs.sample_number variable, which is only ever set when an item also needs lab testing. Adds tnsw/npqs_v2/4-0-visual_sample_collection, a small two-step sub-workflow mirroring 2-sample_collection's shape: trader_submit (USER_INPUT, "confirm you dropped off a sample") -> officer_receive (EXTERNAL_REVIEW, "verify & register the received sample, assign a sample number"). Wired into the parent workflow as a new node n_visual_sample_collect between gw_visual_split's 'sample' edge and n_visual_sample_inspection, with n_visual_sample_inspection now reading sample_number from its own dedicated npqs.visual_sample_number variable instead of the lab track's. Registers the new officer-facing review form + task config (npqs_visual_sample_receive_v2) under the split top-level npqs_v2/ artifact root added last commit - doesn't touch npqs/ or tnsw/npqs at all. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(npqs-v2): register visual-sample-collection in tnsw/manifest.json; simplify to pure status flow; fix output_mapping Two real bugs found by live-testing the previous commit against the running dev stack: 1. tnsw/manifest.json (nsw-srilanka's own artifact catalog, loaded once at process startup, separate from the agency-side npqs_v2/manifest.json added earlier) never got entries for the new npqs-v2-visual-sample-collection task_template/workflow/subtask/render artifacts. The parent workflow's activity kept failing with "artifact not found: npqs-v2-visual-sample-collection/task_template" and retrying forever - no task record was ever created, so the trader saw a permanently blank screen. Fixed by adding the 6 missing rows and restarting tnsw-api to reload the manifest (loaded once at boot, not live like individual artifact content). 2. officer_receive's own output_mapping used "reviewerform.sample_number" as its left-hand key. That's wrong: output_mapping's LHS reads the RAW, un-namespaced activity result (CompleteTaskStep passes the officer's flat reviewerResponse straight through) - the "reviewerform." nesting only applies to record.Data, used for rendering, not to workflow variable mapping. Every other correct EXTERNAL_REVIEW node in this codebase (e.g. 2-sample_collection's own officer_receive) uses a flat key. This bug didn't error loudly - the graph interpreter "parks" a node on a mapTaskOutputs failure instead of failing the Temporal workflow task, so the workflow just silently stopped forever after the officer submitted, with no error surfaced anywhere. Fixed to a flat "sample_number": "sample_number", matching the working pattern. Also simplifies the design per feedback: the trader doesn't need to actively confirm dropping off the sample. Removed the trader_submit USER_INPUT step entirely - the trader now just sees a pure status message ("please drop off a sample" -> "sample received") with no form to fill, exactly matching how a physical drop-off actually works and matching the established pattern used by the other read-only-for-trader tracks (visual system/lab testing) already fixed earlier this session. Added test/npqs/npqs_visual_sample_collection_test.go in nsw-srilanka, which drives this sub-workflow's own workflow.json directly (the existing full-simulation test mocks at too high a level - it treats the whole sub-workflow as one opaque activity call and can't see output_mapping bugs inside it) with the exact flat payload shape NSW Agency actually posts. Confirmed it fails (times out, matching the real parked-workflow symptom) against the old output_mapping and passes against the fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(npqs-v2): show per-item routing on the application-approved status view assessment_outcome's section is dataKey-scoped to "reviewerform" (its render.json), so .lab_required/.visual_required/.treatment_required/ .lab_sample_method/.visual_approach resolved against reviewerform directly - but those are per-item fields the officer sets inside reviewerform.commodities[], not direct properties of reviewerform itself. They were always undefined, so the trader always saw "Not Required" for every inspection path regardless of what was actually configured per item. Only .review_outcome, .reference_number, .ephyto_required, and .clarification_reason are genuinely batch-level reviewerform fields and stay as direct references. Replaced the single aggregate line per path with a per-item breakdown (ranging over .commodities, which resolves to reviewerform.commodities under this section's dataKey), so the trader sees exactly which items need lab testing, visual inspection, or treatment. Verified against this exact bug's real trigger data (a live consignment where item-1/item-2 have visual_required: true) via Go's text/template - correctly shows "Required" for those items now, where it silently showed "Not Required" for everything before. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(npqs-v2): show item context during the trader's own scheduling step status_message (the section carrying the item list + narrative status text) was gated to states: [QUEUED_EXTERNALLY] only - the window after the trader submits, while waiting on the officer. But this sub-workflow's FIRST step (trader_schedule) runs in PENDING_USER, before that gate opens, so a trader landing on "schedule your consignment inspection" saw only the bare date/time/contact form - no indication of which items the inspection even covers, since assessment_outcome (the other item-list-carrying section) is also hidden at that point (gated on reviewerform, which doesn't exist yet either). Added PENDING_USER to status_message's states gate. Its own template already handles this correctly (falls through to the "Schedule Your Consignment Inspection" branch, which lists items, when neither reviewerform.visual_result nor traderinput.inspection_date exist yet) - this was purely a visibility gate gap, not a template content bug. Checked every other track with the same trader-form-then-officer-review shape (upload_docs, ephyto, treatment_upload_certs, treatment_request) for the same gap: none have it. treatment_request has a separate, always-visible items_summary section that already covers this; the other three already include PENDING_USER in their form-carrying section's own gate. Verified live against a completed real task from this session (n_visual_consignment_flow) that the assessment_outcome branch (post- completion) already renders items + per-item pass/fail correctly - this fix only closes the earlier, pre-submission gap. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(npqs-v2): show which items are expected/being tested on lab-sample-collection and lab-testing markdown Two related gaps in the lab track's trader-side status views: 1. The officer's "NPQS Receive Sample" review form (agency-side) had no context at all about which commodities the batch's sample is supposed to cover - just a bare sample-number/comments form. Added a read-only items array (id, commodity name, botanical name) sourced from the sub-workflow's own dispatch data, addable:false/removable:false since this step is all-or-nothing (no per-item outcome tracking needed here, just "here's what to expect"). Renamed the officer_receive node's local input key from "commodities" to "items" to match the established convention (and the trader-app's ApplicationDetailScreen prefill whitelist, which already includes "items" but not "commodities") - updated 2-sample_collection/status_markdown.json's own reference to match, and mirrored the officer form onto the split npqs_v2/ agency-side tree. 2. n2_1_lab_testing's (3-lab_testing) trader-facing status markdown only listed items in the post-completion per-item-results branch - while testing was in progress, the trader had no way to tell which items were actually in the lab. Moved the item list (id, botanical name, quantity) to always render at the top, above both the pending and completed branches, matching the pattern already used for every other track fixed this session. Verified via Go's text/template with realistic pending/completed payloads, and the nsw-srilanka workflow test suite still passes with the 2-sample_collection input_mapping rename. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(npqs-v2): drop the misleading single-value provider/supervision line from treatment-request approval gw_treatment_split only partitions on item.treatment_required - it doesn't sub-partition by provider, so a single n3_treatment_request execution can (and does, per a live report) cover items with different treatment_provider/ treatment_supervision preferences (set per-item back at n1_apply). The officer's own review form for this step captures ONE aggregate provider/ supervision decision, not a per-item one, so the "Treatment Provider" / "Supervision" lines in the approval markdown only ever showed one value - misleadingly implying a single uniform decision when items can differ, and duplicating (with different, conflicting values) what items_summary already shows correctly per item. There's no per-item outcome captured at this step to display instead (the review form has no items array), so per the simpler of the two options this was reported with: removed the two lines, kept the "what happens next" guidance (still accurate - the workflow really does take one single path forward from here, driven by the same treatment_provider variable, regardless of what any individual item's original preference was). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(npqs-v2): route treatment by each item's own provider, not one workflow-wide decision Reported bug: selecting NPQS-station treatment for one item and external- provider treatment for two others in the same batch, only the external task was ever created - all three items went down it. Root cause: treatment_provider_split was an EXCLUSIVE_SPLIT on a single workflow variable (npqs.treatment_provider), set by the officer's own ONE decision on n3_treatment_request's review form - completely ignoring each item's own treatment_provider (already set per-item back at n1_apply, and already shown correctly, per item, in the always-visible items_summary section). The same bug existed one level down: n3_3_1_supervision_split routed supervision-report on a single npqs.treatment_supervision value, so a mixed npqs-provider batch (some items needing supervision, some not) also couldn't route correctly. Converts both to per-item BATCH_SPLIT/JOIN gateways (mirroring lab_result_split's existing pattern), reading item.treatment_provider and item.treatment_supervision directly instead of the single workflow variable: treatment_provider_split (now BATCH_SPLIT on item.treatment_provider) -> npqs items: n3_2_treatment_payment -> n3_3_treatment_cert_issue -> n3_3_1_supervision_split (BATCH_SPLIT on item.treatment_supervision) -> with_supervision: n3_3_2_treatment_supervisor_report -> without: (straight through) -> n3_3_1_supervision_join -> external items: n3_4_treatment_upload_certs (direct) both partitions converge at n3_4_treatment_upload_certs, continue through the shared review/post-treatment-visual chain, and exit through the new treatment_provider_join before rejoining gw_treatment_join. Since the real per-item provider/supervision already exists from n1_apply, n3_treatment_request's own treatment_provider/treatment_supervision/ supervision_officer_type fields were pure duplication of that (and the source of the bug, since the workflow trusted this single redundant decision over the real per-item data) - removed them from the review form (tnsw/npqs_v2 spec copy and the split npqs_v2/ agency copy), and from n3_treatment_request's own output_mapping. Fixed the three trader-facing status views that depended on the now-removed workflow variables (5-1-treatment_request, 5-3-treatment_cert_issue, 5-4-treatment_upload_certs) - replaced single-value provider/supervision claims (which could never be accurate for a mixed batch, and are redundant with the item lists already shown) with guidance that doesn't assume batch-wide uniformity. Verified: extended the workflow test suite with item-7 (external provider) and item-8 (npqs provider, with_supervision) alongside the existing item-3 (npqs, without_supervision) - all in the SAME treatment-required batch. Confirmed via the mock's actual execution counts that npqs-v2-pay-for-treatment/issue-treatment-cert run once for item-3+item-8 only, npqs-v2-treatment-supervisor-report runs once for item-8 only, and npqs-v2-upload-treatment-certs/review-treatment-certs each run twice (once per provider partition) before both converge back into the shared PARALLEL_JOIN steps. Also confirmed the test fails (BATCH_JOIN references non-existent BATCH_SPLIT) when treatment_provider_split is reverted to the old EXCLUSIVE_SPLIT, proving it's a real regression guard for this exact bug. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(npqs-v2): stop leaking both provider partitions' items into treatment sub-tasks BATCH_SPLIT only re-scopes its configured items_variable (commodities); npqs.treatment_items was a separate flat variable set once, before treatment_provider_split, from the trader's full treatment submission — so both the npqs and external partitions carried the *combined* item list downstream, unfiltered, into cert issue, supervisor report, upload certs, and review certs. Repoint all four to the already correctly-scoped `commodities` variable instead, dropping the per-item treatment_type column those screens showed (already visible to both trader and officer once, at treatment-request time) in favor of correct per-partition item lists. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(npqs-v2): route visual inspection results per item, not by batch aggregate visual_sample_result_split, visual_consignment_result_split, visual_system_result_split, and visual_sample_escalated_result_split were EXCLUSIVE_SPLIT gateways reading a single workflow-level "worst outcome across items" value the officer picked for the whole batch — so if one item in a batch failed or needed escalation, every item in that batch was forced down the same path, even ones that individually passed. Convert all four to per-item BATCH_SPLIT/BATCH_JOIN pairs (mirroring the lab and treatment tracks), reading item.visual_result instead. This also required propagating the officer's per-item result array back into `commodities` (via items->commodities output_mapping on each inspection sub-workflow's officer node) since nothing wrote it back before. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(npqs-v2): merge lab/visual/treatment tracks by item ID instead of clobbering gw_par_join now carries parallel_join.merge_by_id: {"commodities": "id"}, wiring NPQS into the shared engine's new PARALLEL_SPLIT/JOIN isolation and merge fix (core commit a81c35e). Previously all three tracks ran as in-process coroutines sharing the same commodities variable directly; whichever track's BATCH_SPLIT/JOIN cycle finished last would overwrite the WHOLE array from its own pre-split snapshot, silently reverting the other tracks' contributions — even sample_test_result and visual_result on items each track itself had just processed. Proven with a live item that's both lab_required and visual_required in nsw-srilanka's test suite: both fields now survive to certificate issuance regardless of which track finishes first. Also renames the one field lab and visual could genuinely both write to the same item — failure_reason -> lab_failure_reason / visual_failure_reason — since no merge, however correct, can arbitrate two different values for the identical field. Everything else the two tracks write is already disjointly named (sample_test_result vs visual_result), so this closes the last real collision surface. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(npqs-v2): mirror failure_reason rename to agency-side officer forms The lab/visual failure_reason -> lab_failure_reason/visual_failure_reason rename only touched the tnsw/ spec copies, missing these 4 agency-side mirrors that officers actually submit through in production — leaving the exact field-name collision the rename was meant to eliminate. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(npqs-v2): pre-fill the trader's prior answer on all three needs_more_info loops None of the three resubmission loops (treatment request, docs upload, treatment cert upload) carried the trader's own prior submission forward on a needs_more_info loop-back — each showed a blank or pristine-reset form, forcing the trader to redo everything even for items the officer never flagged. Treatment request loops within a single sub-workflow instance, so it's fixed by seeding and reading the same persistent path (traderinput.treatment_items) the trader's own output already writes to - pristine on first entry, the trader's latest answer on every resubmit, with no fallback logic needed. Docs upload and treatment cert upload re-enter their outer task node fresh each time (a new sub-workflow instance), so their prior submission is relayed in field-by-field from where the outer graph already stores it (npqs.docs_traderinput / npqs.treatment_traderinput) via prior_<field> pass-through variables. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(npqs-v2): fix stale items_summary and <no value> fields on treatment request Both are regressions from the loop-back prefill fix: n3_treatment_request's outer input_mapping now seeds traderinput.treatment_items instead of a bare commodities key, but two templates were never updated to match. - items_summary_markdown.json still read .commodities (now gone entirely, so the item list rendered empty). Switched to .traderinput.treatment_items, and gated the section to PENDING_USER only in render.json - its own text ("please complete the specification below") only makes sense pre-submission, and traderinput.treatment_items loses several fields once the trader's own submission overwrites it, so showing it after submission was never going to render right anyway. - status_markdown.json's "awaiting review" branch read .traderinput.treatment_type and .traderinput.treatment_duration as flat scalars, but they're per-item fields nested inside traderinput.treatment_items[] - always <no value>. Replaced with a per-item breakdown of what the trader actually submitted. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(npqs-v2): remove LankaPay option from treatment payment, GovPay-only Matches the established pattern already used everywhere else in NPQS (7-payment's own certificate-fee step) and in CDA's payment flow - 5-2-treatment_payment was the only payment step in the whole artifact tree still offering LankaPay. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(npqs-v2): clean up payment markdown and blank upload_docs completion view Both NPQS payment steps (7-payment, 5-2-treatment_payment) rendered their PENDING_PAYMENT fee details as a read-only jsonform, plus a "PAYMENT" projector section the trader-app has no renderer for (frontend only supports FORM/MARKDOWN/REDIRECT — it was silently showing "No renderer for component type: PAYMENT"). Replace both with a single MARKDOWN section matching CDA's payment_details_markdown.json pattern; drop the now-dead instructions_wrapper/payment jsonform files. upload_docs's render.json had no section at all for the COMPLETED state, so the page went blank once the trader submitted documents. Unlike the other trader-facing tasks in this flow, officer review here happens in a separate outer node (n5_2_review_docs), not bundled into this task's own sub-workflow, so this task's own record never learns the review verdict. Add a COMPLETED-state confirmation section showing the reference number and submitted document links instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(npqs-v2): map per-item arrays directly into submission, drop redundant siblings The "submission" reserved key + sibling-merge convention (introduced this session in nsw-srilanka's external_review.go) was unnecessary complexity for a problem input_mapping's own nested dot-path support already solves. Every "commodities?"/"items?"/"certificate_items?" sibling here can instead write straight into a nested submission.<field> path via the engine's existing maputil.SetNestedKey, with no plugin-level merge step needed. Split the 11 affected nodes two ways: - 6 nodes (2-sample_collection, 3-lab_testing, 4-1-visual_sample, 4-2-visual_consignment, 4-3-visual_system, 8-issue_certificate) map a genuinely different per-branch-partitioned array under a destination name (items/certificate_items) that userform/traderinput doesn't already carry — converted to "submission.items"/"submission.certificate_items" directly. - 5 nodes (1-apply, 4-0-visual_sample_collection, 5-3-treatment_cert_issue, 5-5-treatment_review_certs, 5-6-treatment_supervisor_report) had a sibling destined for "commodities", which userform/traderinput already independently carries (the frozen apply-time full commodity list, which is what the shared officer-view-form-v2 schema is actually built to display). The sibling was always redundant here — for 3 of the 5 it was silently discarded by submission's existing precedence anyway; for the other 2 it referenced a field name that doesn't even appear in their own view form's schema. Converting it to a nested path would have introduced a genuine Go-map-iteration-order race against userform's own field instead of fixing anything, so these are simply deleted. Verified deterministic across 200 randomized map-iteration-order runs using the real maputil package. external_review.go's merge code is intentionally untouched — reference_number and other siblings across every agency still depend on it; that's a separate, out-of-scope cleanup. Note: 5-5-treatment_review_certs's view form expects a "treatment_items" field that nothing currently populates (5-4-treatment_upload_certs's output_mapping never sets it) — a separate, pre-existing display gap, not something this change touches. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(npqs-v2): finish weaning external_review nodes off the submission sibling convention Companion to edd6dde: convert every remaining sibling across npqs_v2's external_review nodes now that nsw-srilanka's external_review.go no longer merges siblings into submission at all (see that repo's 0478ccd). Two kinds of cleanup: - reference_number, sample_number (3-lab_testing, 4-1-visual_sample), and userform (5-1-treatment_request) are genuine data that userform/traderinput doesn't already carry — converted to direct "submission.<field>" input_mapping paths. - nppo_office_location (2-sample_collection, 4-0-visual_sample_collection), inspection_date/inspection_time/contact_name/contact_phone (4-2-visual_consignment), treatment_certificate_url/ supervision_report_url/upload_remarks (5-5-treatment_review_certs), and invoice_file_url/packing_list_file_url/additional_file_url (6-2-review_docs) were always redundant: each reads from a nested path already inside whatever gets wholesale-copied into submission, so the sibling was dead weight regardless of merge behavior. Deleted rather than converted. Every external_review node in npqs_v2 now relies on nothing beyond input_mapping's own nested dot-path support. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat(npqs-v2): require certificate_items on certificate issuance Companion to nsw-srilanka's cec1510: the officer can no longer submit this form without touching the item picker. npqs is being deprecated this week in favor of npqs_v2, so there's no longer a reason to let this slide gracefully all the way through to ephyto's SOAP builder — better to reject it right here, at the form the officer is looking at. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat(npqs): replace the single-item flow with the batch workflow, drop v2 naming npqs (old, single-item) is deprecated this week in favor of the batch workflow built under npqs_v2 this cycle. Consolidate to one tree at the plain npqs/ (agency) and tnsw/npqs/ (trader) paths, and strip the now purely historical "v2"/"V2" marker from every artifact id, task_code, form id, and file name across both trees (152 ids renamed, 14 taskconfig files renamed, 14 dead trader-side files removed that turned out to be unreferenced leftovers from before agency review forms had their own copies). Verified via nsw-srilanka's full test/npqs suite, including the static cross-check between every tnsw node's wiring and its agency form schema (catches any missed id/reference automatically) — all green, plus a raw sweep confirming zero remaining "v2" occurrences anywhere in either tree. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(npqs): fix stale tnsw/manifest.json, restore x-search, drop dead duplicates The npqs_v2 -> npqs consolidation (5fb86fc) never updated tnsw/manifest.json — the real, production id->path registry nsw-srilanka's artifact loader actually reads (internal/bootstrap/app.go). It still had 134 entries under the now-nonexistent npqs_v2/ path (100% broken) alongside its original npqs/ section, which itself had drifted: 17 entries pointed at files deleted during cleanup, and 9 newly-added files (4-0-visual_sample_collection, plus this session's new markdown templates) were never registered at all. Rebuilt it to exactly match the actual tree: dropped the dead npqs_v2/ section, removed the 17 stale entries, added the 9 missing ones. Restored x-search on 1-apply/userinput_jsonform.json's importing_country and commodity_common_name fields — these had been replaced with a fully inlined static oneOf list at some point during npqs_v2's development, which is what actually inflated this one file from 715 to 2,471 lines (all the world's countries enumerated inline instead of referenced by id). Restored the two backing static-data files (importing_countries.data.json, commodity_common_names.data.json) that had been deleted along with old npqs, and ported forward the genuinely new content (the per-item id field, added for BATCH_SPLIT tracking) onto the original compact formatting rather than starting from the bloated version. Deleted 9 agency-side userinput_jsonform.json files that all declared the identical id (npqs-apply-phyto-cert--user-form) as the one legitimate tnsw-side copy, registered in npqs/manifest.json, but referenced by zero taskconfigs — ~21,500 lines of pure duplicate dead weight, apparently copy-pasted into each new task directory during npqs_v2 scaffolding and never cleaned up. Net effect: the npqs tree is now 11,068 lines, smaller than the 14,177 lines old npqs itself was — despite genuinely new batch-workflow content. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(npqs): fix commodity_common_name example shape after x-search restore Restoring x-search (83ffa19) made commodity_common_name an object ({value, label}) again, but the commodities.example array I ported forward still had it as a plain string — whatever in the trader app autofills from this example was feeding a bare string into a field that now requires an object, surfacing as "must be object" on the live form. Fix the 4 example entries to match. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(npqs): propagate the x-search object shape everywhere commodity data flows Restoring x-search (25dcbe1) made commodity_common_name and importing_country {value, label} objects again at the point of entry (1-apply), but every downstream consumer of that same data — 12 agency review/view schemas, 2 more trader-side forms, and 14 status markdown templates — still declared/rendered them as plain strings. In practice this broke injection outright: the agency rejected any submission with "type: ... has type \"object\", want \"string\"" the moment an officer task tried to validate the trader's data against its own schema. Fixed all schema declarations to the same {value, label} object shape, and all `{{.commodity_common_name}}`/`{{.importing_country}}` markdown references to `.label` so they render the human name instead of Go's default map formatting. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(trade): remove leftover npqs-v2 option from HS code selection The npqs -> npqs_v2 -> npqs consolidation (83ffa19) never touched tnsw/trade/, so its HS-code-to-workflow mapping still offered a "npqs-v2-export-phytosanitary-reg" option alongside the real "npqs-export-phytosanitary-reg" — a dead id nothing produces anymore, left selectable in the hscode_selection form, the split-items transform mapping, and the summary markdown's display branch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(npqs): add x-search to every downstream commodity_common_name/importing_country field d7a3498 converted these fields to {value, label} objects everywhere they flow, but that alone isn't enough for ui-packages' renderer set to actually render them: SearchSelectControlTester only matches a type: "object" field if it also carries x-search.service — a plain object with no x-search hits zero renderers, surfacing as "No applicable renderer found" on every officer-side form displaying batch item context. Add the same x-search config used on the original 1-apply input field to all 18 downstream occurrences (they're all read-only context displays or already-submitted values, so this only needs to satisfy the renderer's tester and let it show the already-known label — SearchSelectControl does that without touching the search service at all when data already carries {value, label}). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(npqs): show per-item treatment plan on treatment-request awaiting status The status-awaiting template merged in from main (#54) referenced traderinput.treatment_type/treatment_duration as top-level fields, but the batch refactor moved them under traderinput.treatment_items[]. This is the only section visible while a request sits in QUEUED_EXTERNALLY, so traders saw two blank lines and no item list while awaiting review. Match the per-item rendering already used in status_markdown.json's own awaiting branch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(npqs): fix broken treatment-items display on treatment request officer view The officer's Application Context view nested treatment_type/chemical_used/ treatment_duration/treatment_temperature/treatment_concentration/ treatment_items under a "traderinput" object, but the officer_review TASK node's input_mapping (tnsw/npqs/5-1-treatment_request/workflow.json) maps "traderinput": "submission" as a whole-namespace mapping, which the runtime flattens into the root data object rather than nesting it under a "traderinput" key (confirmed against the working fcau-warehouse-inspection precedent, and against the actual API response, which has treatment_items at the root with no traderinput wrapper at all). The officer therefore saw "No items have been added yet." and five blank fields. Move treatment_items to the schema root to match where the data actually lands, and drop the five singular treatment_type/chemical_used/... fields entirely -- they predate the per-item batch refactor and were never populated at any level; the same data now lives inside each treatment_items entry, which the (previously unreachable) per-item schema already covers. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(npqs): label treatment type/duration on treatment-request status views Submitted Treatment Plan line showed raw codes with no field names ("CD, a"), making it unclear which value was which. Label both. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves https://github.com/LSFLK/lsf-govtech-tnsw/issues/103
Recreation of #52 on latest
main(#49 is already merged). Same defect class on other same-stepneeds_more_infoloops: after the trader resubmits,QUEUED_EXTERNALLYstill showed leftover officer “please correct / resubmit” copy as current status.Summary
PENDING_USER/COMPLETED: action-required or terminal assessment (deficiencies, approve).QUEUED_EXTERNALLY: Current Status (submitted vs resubmitted, awaiting officer). Prior officer comments move to Review History.Not changed: split-step loops (NPQS docs upload↔review, treatment-cert upload↔review), SLTB levy, CDA lot adjustment, SLTB pickup scheduling — those already hide correction copy except when the trader can act.
Test plan
QUEUED_EXTERNALLYshows submitted / awaiting; no deficiency banner; no Review Historyneeds_more_info→PENDING_USERshows deficiencies / “please resubmit”QUEUED_EXTERNALLYshows resubmitted / awaiting; officer comments only under Review HistoryCOMPLETEDshows the terminal assessment, not the old correction copy